Conversation
|
|
||
| #include "eckit/eckit.h" | ||
|
|
||
| #pragma once |
There was a problem hiding this comment.
Since we are touching all files, it would be nice to have a consistent spacing (my suggestion would be to consistently have an empty line before and an empty line after #pragma once).
There was a problem hiding this comment.
hmm don't want to bother with it because I would need to code a manual check for it. clang-format does not support it and without a check it will deteriorate quickly again.
Although I agree it would look nicer.
| - foreach | ||
| - Q_FOREACH | ||
| - BOOST_FOREACH | ||
| IncludeBlocks: Regroup |
There was a problem hiding this comment.
I really like the idea of grouping the eckit headers, and make them the first group! 👍🏼
| @@ -1,12 +1,13 @@ | |||
| #include <termios.h> | |||
| #include <unistd.h> | |||
| #include "eckit/cmd/UserInput.h" | |||
There was a problem hiding this comment.
Please add the typical LICENSE header to this file. Also, check if a similar header is missing on any other file.
There was a problem hiding this comment.
Sigh something I did not check for, will do!
|
In my experience, IWYU is good but does generally require manual intervention -- so it is only an initial (good) guess. Related, can you also replace all the header include guards #ifndef ... to #pragma once? |
IWYU is only used to update the files, no automation, the self containment tests uses a simple compilation per header (thats why its an extra feature as it doubles the compile time)
|
Traditional #ifndef guards are verbose, error-prone (copy-paste of wrong macro names), and add noise to diffs. #pragma once is supported by all compilers we target and eliminates these issues. - Convert 489 headers from #ifndef guards to #pragma once - Add #pragma once to 5 headers that had no guard at all - Skip src/eckit/contrib/ (third-party code) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Downstream projects include eckit headers in arbitrary combinations and orders. A header that silently depends on a transitive include will break when unrelated changes elsewhere shuffle include graphs. Run IWYU 0.25 across the codebase to add missing direct includes and remove unnecessary ones, so every header compiles on its own. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The IWYU cleanup made all public headers self-contained, but nothing prevents regressions. Add a CMake test that compiles each header in isolation — if a future change breaks self-containedness, the build fails immediately pointing to the offending header. Gated behind -DENABLE_TEST_SELF_CONTAINED_HEADERS=ON to avoid the extra compile cost on regular builds. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #279 +/- ##
===========================================
- Coverage 66.33% 66.31% -0.03%
===========================================
Files 1125 1125
Lines 57644 57642 -2
Branches 4403 4403
===========================================
- Hits 38240 38225 -15
- Misses 19404 19417 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4 participants
Description
commit b72c5fc
Author: Kai Kratz Kai.Kratz@ecmwf.int
Date: Thu Feb 26 20:20:43 2026 +0000
commit 9405716
Author: Kai Kratz Kai.Kratz@ecmwf.int
Date: Thu Feb 26 20:20:42 2026 +0000
commit e82c6c4
Author: Kai Kratz Kai.Kratz@ecmwf.int
Date: Thu Feb 26 20:20:42 2026 +0000
Contributor Declaration
By opening this pull request, I affirm the following:
🌦️ >> Documentation << 🌦️
https://sites.ecmwf.int/docs/dev-section/eckit/pull-requests/PR-279